Skip to content

Conversation

@mashajuventus
Copy link

Реализованы поиск по письмам и темная тема.

@vercel
Copy link

vercel bot commented Jun 24, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://task-5-git-fork-mashajuventus-maria-popyrkina-task-6.itmo-yandex.now.sh

"not op_mini all"
],
"dependencies": {
"@types/classnames": "2.2.8",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm install --save-dev @types/*

height: 600px;
background-color: #e5eaf0;
margin: 0;
/*background-color: black;*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не оставляй мёртвый код

margin-right: 20px;
}

.inp {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо бы добавить немного творчества при именовании переменных)

@@ -0,0 +1,27 @@
import React, { Component } from 'react';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Импорты можно делить на группы:

  1. Встроенные модули в браузер (таких пока нет) / батарейки Node.js
  2. Сторонние библиотеки
  3. JS импорты внутри проекта
  4. Импорт ресурсов (css, font, images)

import { Header } from '../header/Header';
import { Menu } from '../menu/Menu';
import { LettersList } from '../lettersList/LettersList';
import { ILetter } from '../letter/ILetter';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно рядом держать файлик types.ts

template: string;
}

export class Main extends Component<{}, IState> {
Copy link

@evgenymarkov evgenymarkov Jun 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно так:

public readonly state: IState = {
  isDark: false,
  letters: [],
  openId: -1,
  template: ''
}

this.searchLetters = this.searchLetters.bind(this);
this.setLetters = this.setLetters.bind(this);
this.changeColor = this.changeColor.bind(this);
this.curMin = 10;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно вынести из конструктора в поле класса.
И назвать чуть понятнее :)

public static CUR_MIN = 10;

openId: -1,
template: ''
};
this.letterAdded = this.letterAdded.bind(this);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно удалить т.к ты используешь стрелочные функции сохранённые в поля класса

isDark: false,
letters: [],
openId: -1,
template: ''

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убирай мёртвый код

openId={this.state.openId}
openLetters={this.openLetters}
markNotNew={this.markNotNew}
isDark={this.state.isDark}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для того чтобы в дерево компонентов глубоко прокидывать пропсы можно использовать контексты

<Menu />
<LettersList
letterAdded={this.letterAdded}
letterChose={this.letterChose}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onLetterChose

lettersS[i].isVisible =
lettersS[i].letterText.includes(template) || lettersS[i].sender.includes(template);
}
this.setState({ template: template, letters: lettersS });
Copy link

@evgenymarkov evgenymarkov Jun 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

template: template -> template

Copy link

@evgenymarkov evgenymarkov Jun 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lettersS -> letters

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.setState(oldState => {
  // Your code
});

const lettersS = this.state.letters;
for (let i = 0; i < lettersS.length; i++) {
if (lettersS[i].id === id) {
lettersS[i].classS = "notNew";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Держи в голове, что изменение чего-то внутри стейта может привести к сложновоспроизводимым багам

delay += genDelay;
}
this.diff = delay;
console.log(delay);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ата-та


import styles from './Menu.module.css';

export class Menu extends Component {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Попробуй функциональные компоненты, для начала в местах где есть только вёрстка без логики

openId: number;
letters: ILetter[];
openLetters: () => void;
markNotNew: (a: number) => void;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id or index

allLettersChose={this.props.allLettersChose}
changeColor={this.props.changeColor}
/>
<ul className={styles.lettersList}>{code}</ul>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Внутри ul не может быть других детей кроме li

return (
<section className={darkClass}>
<LetterHeader
letterAdded={this.props.letterAdded}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const { lettersDeleted, allLettersChose } = this.props;

}

export class LetterHeader extends Component<IProps> {
public constructor(props: IProps) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private markerRef = React.createRef();

private markerRef: RefObject<HTMLInputElement>;

public render() {
const sqBut = cn(styles.squareForButton, stylesHead.headerPart);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sqBut

<input
ref={this.markerRef}
type="checkbox"
name="CCC"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

type="checkbox"
name="CCC"
className={styles.inputButton}
onClick={() => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно вынести в отдельный обработчик в классе, никаких доп. аргументов ты в обработчики не передаёшь

name="resendButton"
value="Переслать"
className={stylesHead.headerButtons}
onClick={() => this.props.letterAdded(0)}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тоже можно вынести

/>
</li>
<li className={stylesHead.headerPart}>
<input

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

button type="button"

outline: none;
background-color: inherit;
font-weight: 500;
color: #cccccc;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно сокращать до #ccc

}, // .bind(this)
1000);
} else if (this.props.classS === 'delete') {
this.markerRef.current.className = styles.letter_delete;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Юзай classList

import styles from './Letter.module.css';

interface IProps {
classS: string;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cls, modifier

<input
type="checkbox"
className={styles.inputButton}
onChange={() => this.props.letterChose(this.props.id)}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Постарайся сокращать использование lambda функций в render, это плохо влияет на производительность

/*background-color: black;*/
}

.selectButton:after {
Copy link

@evgenymarkov evgenymarkov Jun 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отделяй псевдоэлементы от псевдоклассов
::

border-right: 2px solid;
border-bottom: 2px solid;
color: #000000;
-webkit-transform: scale(0.85) rotate(47deg) skewX(12deg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть тулза, которая сама проставляет префиксы: Autoprefixer

.letterDate {
float: right;
margin-right: 20px;
font-family: HelveticaNeueCyr-Medium;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо писать fallback

}

.letterDate {
float: right;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот это древности)) Используй float только для обтекания

src: url('./sourse/fonts/HelveticaNeueCyr-Medium.eot?#iefix') format('embedded-opentype'),
url('././sourse/fonts/HelveticaNeueCyr-Medium.svg#HelveticaNeueCyr-Medium') format('svg'),
url('././sourse/fonts/HelveticaNeueCyr-Medium.ttf') format('truetype'),
url('././sourse/fonts/HelveticaNeueCyr-Medium.woff') format('woff'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Актуальны только woff2, woff

<img
className={styles.yandexMail_picture}
alt="yandexMailPicture"
src={actualLogo}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно было прост isDark ? darkLogo : logo

@@ -0,0 +1,52 @@
const senders = ['Mom', 'Dad', 'Cat', 'Dog', 'Apple', 'Teacher', 'Homie', 'Mole',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsx -> ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants